-
Notifications
You must be signed in to change notification settings - Fork 57
Don't hold database connections open when talking to the homeserver #4527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
sandhose
commented
May 7, 2025
- Introduce a RepositoryFactory
- Move the pool acquisition metric logic to the PgRepositoryFactory
- Use the new RepositoryFactory everywhere
- Don't hold db conns when creating a device on the compat login API
Deploying matrix-authentication-service-docs with
|
Latest commit: |
8d7be72
|
Status: | ✅ Deploy successful! |
Preview URL: | https://904c698e.matrix-authentication-service-docs.pages.dev |
Branch Preview URL: | https://quenting-dont-hold-db-conns.matrix-authentication-service-docs.pages.dev |
17e3732
to
8d7be72
Compare
{ | ||
// Something went wrong, let's end this session and schedule a device sync | ||
let mut repo = repository_factory.create().await?; | ||
let session = repo.compat_session().finish(&clock, session).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one slight difference in behaviour now: if the device creation failed on Synapse, before we'd just rollback the transaction, meaning that the client could retry the request with the same login token and it would work. This isn't the case anymore
It may also end another session, since we now end sessions with the same device ID
To be fair, this request was never really idempotent, so if the client lost the response it would have had the same effect
A build for this PR at commit 1091b51 has been created through the Z-Build-Workflow label by sandhose. Docker image is available at:
Pre-built binaries are available through the workflow run artifacts. |
A build for this PR at commit 79a4673 has been created through the Z-Build-Workflow label by sandhose. Docker image is available at:
Pre-built binaries are available through the workflow run artifacts. |
crates/storage/src/repository.rs
Outdated
}; | ||
|
||
/// A [`RepositoryFactory`] is a factory that can create a [`BoxRepository`] | ||
// XXX(quenting): this could be generic over the repository type, but it's annoying to make it dyn-safe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh, don't overcomplicate it any more than it needs to be. For this, I don't think we should worry about the boxing